Problem: bulk-add-genesis-account is not used in testground #1582
Conversation
Solution: - extract standalone changes from crypto-org-chain#1575 - use a simple and deterministic way to generate test accounts
yihuang
requested review from
devashishdxt and
thomas-nguy
and removed request for
a team
WalkthroughThe pull request includes updates to the Changes
Possibly related PRs
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1582 +/- ##
===========================================
+ Coverage 15.24% 36.12% +20.88%
===========================================
Files 67 97 +30
Lines 4874 7725 +2851
===========================================
+ Hits 743 2791 +2048
- Misses 4037 4585 +548
- Partials 94 349 +255 |
mmsqe
left a comment
mmsqe
left a comment
There was a problem hiding this comment.
seems validator doesnt generate when validator_generate_load is false?
WDYM? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- testground/benchmark/benchmark/peer.py (6 hunks)
- testground/benchmark/benchmark/types.py (1 hunks)
- testground/benchmark/benchmark/utils.py (4 hunks)
Additional context used
Ruff
testground/benchmark/benchmark/utils.py
4-4:
itertools.dropwhileimported but unusedRemove unused import
(F401)
4-4:
itertools.takewhileimported but unusedRemove unused import
(F401)
GitHub Check: Lint python
testground/benchmark/benchmark/utils.py
[failure] 4-4:
./testground/benchmark/benchmark/utils.py:4:1: F401 'itertools.dropwhile' imported but unused
[failure] 4-4:
./testground/benchmark/benchmark/utils.py:4:1: F401 'itertools.takewhile' imported but unused
Additional comments not posted (8)
testground/benchmark/benchmark/types.py (3)
8-13: LGTM!The
Balanceclass is well-structured and provides a clear representation of a balance with amount and denomination. The use ofpydantic'sBaseModelensures proper validation and serialization. The__str__method is a nice addition for convenient string formatting.
18-18: Improved data modeling with thecoinsattribute.The change from
balancetocoinsis a significant improvement in the data modeling of theGenesisAccountclass. It allows for a more flexible and accurate representation of account balances by supporting multiple denominations. The use of a list ofBalanceinstances provides a structured and type-safe approach.
18-18: Verify the usage of thecoinsattribute in the codebase.Ensure that all references to the
balanceattribute are updated to use the newcoinsattribute and handle the list ofBalanceinstances appropriately.Run the following script to verify the usage of the
coinsattribute:testground/benchmark/benchmark/utils.py (3)
14-15: LGTM!Defining the Cronos address prefix as a constant is a good practice. It improves code readability and maintainability.
98-100: LGTM!The
eth_to_bech32function provides a useful utility for converting Ethereum addresses to Bech32 format. The implementation looks correct and follows the Bech32 encoding process.
137-142: LGTM!The
gen_accountfunction provides a deterministic way to generate test accounts based on a global sequence and an index. This is useful for testing and benchmarking purposes. Reserving index 0 for a validator account is a good practice to ensure consistency across tests.testground/benchmark/benchmark/peer.py (2)
Line range hint
64-87: LGTM! The changes improve account management efficiency.The updates to the
init_nodefunction enhance the efficiency and adaptability of account management during node initialization:
- The
global_seqparameter ensures unique accounts are generated for each participant.- The
gen_accountfunction generates Ethereum-compatible addresses for additional accounts.However, please note that the unsafe import method used for the validator account provides greater flexibility but may have security implications. Ensure that the risks associated with this approach are carefully considered and mitigated.
120-128: Excellent optimization! The use of a temporary file for bulk account addition significantly improves efficiency.The modification to the
gen_genesisfunction to utilize a temporary file for bulk account addition is a great optimization. This change streamlines the process of adding multiple genesis accounts in a single command, reducing the number of CLI calls required during the genesis block creation.This enhancement significantly improves the efficiency of the genesis block generation process.
2 participants
Solution:
👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻
PR Checklist:
make)make test)go fmt)golangci-lint run)go list -json -m all | nancy sleuth)Thank you for your code, it's appreciated! :)
Summary by CodeRabbit
New Features
Balanceclass for improved representation of account balances.Bug Fixes
Documentation